From: Daniel Golle <daniel@makrotopia.org>
Date: Fri, 6 Nov 2020 18:42:25 +0000 (+0000)
Subject: jail: fix capabilities
X-Git-Url: http://git.openwrt.org/%22https:/collectd.org//%22http:/www.crowdsec.net/%22/%22https:/collectd.org/%22http:/www.crowdsec.net/%22?a=commitdiff_plain;h=b0de894830a93da5b303a3d89a42baf163d0a58c;p=project%2Fprocd.git

jail: fix capabilities

Allocate enough stack space for capget()/capset() which requires
2*sizeof(struct __user_cap_data_struct), each containing 32-bit fields,
where the 2nd struct contains the bits for high (>32) capabilities.
Failing to do that not only leads to those high capabilities being
inaccessible but also overwrote the stack resulting in ujail hanging
infinitely instead of returning from applyOCIcapabilities().
Also adapt debugging output to 64-bit format.
Apart from that, don't set SECBIT_NO_SETUID_FIXUP when not actually
modifying capabilities explicitely, as that would result in ALL
capabilities retained in the subsequent setuid() call instead of
having them all dropped.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---

diff --git a/jail/capabilities.c b/jail/capabilities.c
index b362746..434fc2f 100644
--- a/jail/capabilities.c
+++ b/jail/capabilities.c
@@ -120,7 +120,7 @@ int parseOCIcapabilities(struct jail_capset *capset, struct blob_attr *msg)
 int applyOCIcapabilities(struct jail_capset ocicapset, uint64_t retain)
 {
 	struct __user_cap_header_struct uh = {};
-	struct __user_cap_data_struct ud;
+	struct __user_cap_data_struct ud[2];
 	int cap;
 	int is_set;
 
@@ -153,25 +153,37 @@ int applyOCIcapabilities(struct jail_capset ocicapset, uint64_t retain)
 	uh.version = _LINUX_CAPABILITY_VERSION_3;
 	uh.pid = getpid();
 
-	if (capget(&uh, &ud)) {
+	if (capget(&uh, ud)) {
 		ERROR("capget() failed\n");
 		return -1;
 	}
 
-	DEBUG("old capabilities: Pe=%08x Pp=%08x Pi=%08x\n", ud.effective, ud.permitted, ud.inheritable);
+	DEBUG("old capabilities: Pe=%016llx Pp=%016llx Pi=%016llx\n",
+		0LLU | ud[0].effective | (0LLU | ud[1].effective) << 32,
+		0LLU | ud[0].permitted | (0LLU | ud[1].permitted) << 32,
+		0LLU | ud[0].inheritable | (0LLU | ud[1].inheritable) << 32);
 
-	if (ocicapset.effective != JAIL_CAP_ALL)
-		ud.effective = ocicapset.effective | retain;
+	if (ocicapset.effective != JAIL_CAP_ALL) {
+		ud[0].effective = (ocicapset.effective | retain) & 0xFFFFFFFFU;
+		ud[1].effective = ((ocicapset.effective | retain) >> 32) & 0xFFFFFFFFU;
+	}
 
-	if (ocicapset.permitted != JAIL_CAP_ALL)
-		ud.permitted = ocicapset.permitted | retain;
+	if (ocicapset.permitted != JAIL_CAP_ALL) {
+		ud[0].permitted = (ocicapset.permitted | retain) & 0xFFFFFFFFU;
+		ud[1].permitted = ((ocicapset.permitted | retain) >> 32) & 0xFFFFFFFFU;
+	}
 
-	if (ocicapset.inheritable != JAIL_CAP_ALL)
-		ud.inheritable = ocicapset.inheritable;
+	if (ocicapset.inheritable != JAIL_CAP_ALL) {
+		ud[0].inheritable = (ocicapset.inheritable | retain) & 0xFFFFFFFFU;
+		ud[1].inheritable = ((ocicapset.inheritable | retain) >> 32) & 0xFFFFFFFFU;
+	}
 
-	DEBUG("new capabilities: Pe=%08x Pp=%08x Pi=%08x\n", ud.effective, ud.permitted, ud.inheritable);
+	DEBUG("new capabilities: Pe=%016llx Pp=%016llx Pi=%016llx\n",
+		0LLU | ud[0].effective | (0LLU | ud[1].effective) << 32,
+		0LLU | ud[0].permitted | (0LLU | ud[1].permitted) << 32,
+		0LLU | ud[0].inheritable | (0LLU | ud[1].inheritable) << 32);
 
-	if (capset(&uh, &ud)) {
+	if (capset(&uh, ud)) {
 		ERROR("capset() failed\n");
 		return -1;
 	}
diff --git a/jail/jail.c b/jail/jail.c
index 4b368ea..f09e1a4 100644
--- a/jail/jail.c
+++ b/jail/jail.c
@@ -1178,9 +1178,15 @@ static void post_start_hook(void)
 {
 	int pw_uid, pw_gid, gr_gid;
 
-	if (prctl(PR_SET_SECUREBITS, SECBIT_NO_SETUID_FIXUP)) {
-		ERROR("prctl(PR_SET_SECUREBITS) failed: %m\n");
-		exit(EXIT_FAILURE);
+	/*
+	 * make sure setuid/setgid won't drop capabilities in case capabilities
+	 * have been specified explicitely.
+	 */
+	if (opts.capset.apply) {
+		if (prctl(PR_SET_SECUREBITS, SECBIT_NO_SETUID_FIXUP)) {
+			ERROR("prctl(PR_SET_SECUREBITS) failed: %m\n");
+			exit(EXIT_FAILURE);
+		}
 	}
 
 	/* drop capabilities, retain those still needed to further setup jail */
@@ -1200,9 +1206,12 @@ static void post_start_hook(void)
 	if (opts.set_umask)
 		umask(opts.umask);
 
-	if (prctl(PR_SET_SECUREBITS, 0)) {
-		ERROR("prctl(PR_SET_SECUREBITS) failed: %m\n");
-		exit(EXIT_FAILURE);
+	/* restore securebits back to normal */
+	if (opts.capset.apply) {
+		if (prctl(PR_SET_SECUREBITS, 0)) {
+			ERROR("prctl(PR_SET_SECUREBITS) failed: %m\n");
+			exit(EXIT_FAILURE);
+		}
 	}
 
 	/* drop remaining capabilities to end up with specified sets */